Add robust error dialog for when digest cycle is broken #2977
Conversation
|
This isn't the only possible approach. We could just make the entire error dialog not depend on Angular, but that seemed more challenging, and this is an edge case. I actually kind of like the fact that when things go this wrong that nothing on the page is going to work anymore, the error dialog appears significantly different. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2977 +/- ##
==========================================
- Coverage 82.08% 82.02% -0.07%
==========================================
Files 544 544
Lines 31644 31673 +29
Branches 5125 5149 +24
==========================================
+ Hits 25976 25979 +3
- Misses 4913 4927 +14
- Partials 755 767 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I've tested on both branches the mentioned steps. Is there a way to test the dialog closes if the onInit completes after 100ms? Also to prevent fewer false positives, should we make it 500ms?
Reviewable status: 0 of 1 files reviewed, all discussions resolved
|
@kylebuss Yes. Add an artificial delay, or reduce the delay down to 0, so it will always mistakenly open. 100ms is a long time in computer terms. I think the system would have to be briefly frozen, and if it's frozen, 500ms probably isn't going to make the difference. |
There was a problem hiding this comment.
That's a fair point. Approving as is.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @Nateowami)
When there's an error in every single digest cycle, the page is completely broken, and not even the error dialog can display. This change adds an extra robust error dialog that is only shown when the normal error dialog is not capable of opening.
The simplest way to cause this to happen is to add an error to a template that will occur every single time. This can be done by adding this to the template:
{{ throwError() }}And adding this to the associated component:
The result is that the page fails to update, and is covered with a dialog backdrop, but no actual error dialog. Depending on where the error occurs, the page may be completely rendered and just not able to update, or barely rendered at all:
This change detects this type of situation by listening for the
onInitevent of the error dialog, and if it hasn't fired within 100ms, assumes the page is broken, and opens a backup error dialog, which is significantly more limited, but more reliable and not dependent on Angular to work. This detection method can have false positives (e.g. performance issues may cause the error dialog to take more than 100ms to open), so if theonInitevent fires after the backup dialog has been opened, it is closed to prevent duplicate dialogs.To test this change, make the described changes to a component, and observe the behavior on this branch and on master.
This change is